-
Notifications
You must be signed in to change notification settings - Fork 148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix the stpg command error for multiple port group #475
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: tangwenji <tang.wenji@zte.com.cn>
tcmu_dev_err(dev, "Failing STPG for group %d. Unable to transition remote groups.\n", | ||
id); | ||
goto free_buf; | ||
tcmu_dev_dbg(dev, "Port of group %d is disable.\n", id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure you want this behavior? We will be reporting success even though we did not transition the state. It could result in a initiator thinking failover worked when it didn't or in some loop where STPG reports success but then a RTPG reports states that do not match what the STPG was requesting, so the initiator retries over and over.
I think I could go either way, because I think we only see this type of command in testing, so I do not really know what the initiator expects and I do not see anything in SPC about this partial case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want this behavior, just make the message above tcmu_dev_warn() in case we find out we were wrong and then this issue will not be silently dropped.
tcmu_dev_warn(dev, "Port is disabled. Not able to change ALUA group %u state to state %u.\n",
group->id, new_state);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like my example,the stpg command is successful,because the second loop meet the criteria。
In the actual environment, the TPG corresponding to the Initiator sd device is enable.So we don't need to log the message with warn level, and if no TPG match,there is error message following。
If there is no enabled TPG, it will be returned before this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like my example,the stpg command is successful,because the second loop meet the criteria。
What about users/apps that expect all transitions requested in the STPG to be successful or the command to fail?
-
Are you saying SPC is ok with partial completions like in your patch? I did not see anything in SPC. If you see something then send the section number.
-
Or, are you saying apps/users are ok with it? My concern was that apps/users expect all transitions in the STPG to succeed if the STPG is returned with a successful status. Is there a specific app that works like how you are testing with sg_stpg or is this just a protocol type of test to check the behavior?
I get what you are saying that for your example we matched one of the group transitions in the STPG, but I was saying the user needs some sort of notification that the entire command is not succeeding for some reason.
When there are multiple target port group,the stpg comman send by the SCSI Initiator carries all target port group information via sg_stpg in Linux。
For example:there are 2 TPGs(id:4,5),the TPG 4 is disable and the TPG 5 is enable. Generate an sdb device via the TPG 5 on the Linux Initiator. The information contained in cdb of sg_stpg /dev/sdb command as follows: 02 00 00 04 00 00 00 05. In the original code, when parsing the first TPG descriptor, it will exit abnormally because group 4 not equal enable port group 5.
Signed-off-by: tangwenji tang.wenji@zte.com.cn